fix(graphql-server): exempt schema introspection from the depth/cost gate - #1817
Conversation
…ost gate The standard introspection document nests __schema > types > fields > args > type > ofType x7 (depth 13), which the default max_query_depth of 12 rejects, so every codegen run against a protected endpoint failed with QUERY_TOO_DEEP. Introspection is governed by enable_introspection alone; its selections carry no connections and are not walked.
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Review complete. 🟡 1 medium 💬 Inline comments (1)
This change tightens the document gate's handling of introspection. The walker in
Reviewed commit: 6c737cb |
There was a problem hiding this comment.
PR #1817 refines the GraphQL document protection gate so schema introspection (__schema/__type) selections are skipped entirely, exempting them from depth and cost budgets while keeping the enableIntrospection switch authoritative.
Key findings
- 🟡 Introspection
continuebypasses the depth budget — document-gate.ts:129
| if (!walk.protection.enableIntrospection) { | ||
| reject(errors.INTROSPECTION_DISABLED()); | ||
| } | ||
| continue; |
There was a problem hiding this comment.
🟡 security · medium
Introspection continue bypasses the depth budget
At graphql/server/src/protection/document-gate.ts:129, the new continue skips the entire __schema/__type selection subtree, so it is never charged depth or cost. Because the introspection schema is recursive (__Type.fields → __Field.type → __Type), a client can send an arbitrarily deep introspection document that the gate reports as { depth: 1, cost: 0 }, defeating the maxQueryDepth DoS protection; only the statement timeout still bounds it.
📋 Prompt for AI Agents
In graphql/server/src/protection/document-gate.ts around line 129, the continue after the __schema/__type introspection check skips the entire selection subtree, so a recursive introspection document (__schema { types { fields { type { fields { type { ... } } } } } }) bypasses maxQueryDepth and is reported as depth 1. Replace the bare continue with logic that still walks the introspection selection set but caps the depth contributed by introspection recursion (for example, recurse into the __schema/__type selection set with a fixed depth allowance independent of maxQueryDepth, or track an introspection-specific depth counter and reject when it exceeds a bounded constant). Keep exempting introspection from cost, but do not let it escape depth bounding entirely.
Summary
Fixes constructive-hub
propagate-to-dashboard(failing every run since Sep 1, e.g. https://github.com/constructive-io/constructive-hub/actions/runs/34269003613):apps/admincodegen dies withFailed to fetch schema: The query is nested too deeply.Since #1753 the document gate charges introspection against the tenant's
max_query_depth. The codegen'sSCHEMA_INTROSPECTION_QUERY(and graphql-js'sgetIntrospectionQuery) nests__schema → types → fields → args → type → ofType×7= depth 13, over the default of 12 — so any protected endpoint refuses introspection.Change in
walkSelectionSet:if (name === '__schema' || name === '__type') { if (!protection.enableIntrospection) reject(INTROSPECTION_DISABLED); + continue; // introspection selections are not walked for depth/cost }Why this rather than raising the default: introspection already has its own switch (
enable_introspection), its types carry no connections so cost is always 0, and its shape is fixed by the spec rather than by the client — the depth budget exists to bound data queries, and a tenant lowering it for their API should not silently lose introspection. Raising the default would only move the cliff.Sibling fields in the same operation are still measured (test added).
Link to Devin session: https://app.devin.ai/sessions/1c0e1e05988944c2995bd82bb7e0c215
Open in Devin Desktop: https://app.devin.ai/desktop/session/1c0e1e05988944c2995bd82bb7e0c215?variant=devin
Requested by: @pyramation